Enhanced OGC reader #2191

mcuntz · 2023-06-11T17:08:03Z

Added support for general EPSG projections in OGC clients WMSRasterSource, WMTSRasterSource, and WFSGeometrySource.

Try to determine geometries from other map servers than the hard-coded mapserver.gis.umn.edu.

Rationale

I tried to use the WFS support with a French server, which did not work because:

the server used the projection EPSG:2154, and
after implementing support for cartopy.crs.epsg, it still did not plot my desired features.

So I

implemented cartopy.crs.epsg as an addition to the known projections. There were only 7 projections known to the OGC readers. Now, all EPSG codes should work.
I try to guess the geometries of a given server because the map server was, strangely, hard-coded to be http://mapserver.gis.umn.edu/mapserver and the geometries were named msGeometry. I search the basic url in the features now and assume that the geometries have geom or Geom in their name. This is not perfect but I do not know any better; this is the first time I use web services for shapes.
While implementing another test case in mpl/test_features.py, I also changed the test case for natural earth from ax.add_feature(cfeature.BORDERS, linestyle=':') to ax.add_feature(cfeature.BORDERS, linestyle='-') because this failed on my machine only due to slightly different dots. Should be more robust know.

Implications

I stayed with the known projections in _URN_TO_CRS and simply added EPSG on top. I had first used the information in cartopy.crs.epsg but the information in EPSG:3031 (Antarctica) was weird and wrong in my opinion.

Guessing the geometries is a layman coding but should be o.k. until it does not work with another server ;-)

mcuntz · 2023-06-11T20:52:57Z

Just took back the change ax.add_feature(cfeature.BORDERS, linestyle=':') to ax.add_feature(cfeature.BORDERS, linestyle='-') for the test case of natural earth because tests fail on github otherwise.

mcuntz · 2023-06-11T22:06:41Z

flake8 checks fail on

            if ((str(default_urn) not in _URN_TO_CRS) and
                (':EPSG:' not in str(default_urn))):
                raise ValueError(
                    f'Unknown mapping from SRS/CRS_URN {default_urn!r} to '
                    'cartopy projection.')

with
lib/cartopy/io/ogc_clients.py:747:17: E129 visually indented line with same indent as next logical line
If I put in some spaces like

            if ( (str(default_urn) not in _URN_TO_CRS) and
                 (':EPSG:' not in str(default_urn)) ):
                raise ValueError(
                    f'Unknown mapping from SRS/CRS_URN {default_urn!r} to '
                    'cartopy projection.')

flake8 also fails complaining that there is a space after '('.

I don't know how to resolve this :-(

akrherz · 2023-06-11T22:32:53Z

Try this

    if (str(default_urn) not in _URN_TO_CRS) and (
        ":EPSG:" not in str(default_urn)
    ):
        raise ValueError(
            f"Unknown mapping from SRS/CRS_URN {default_urn!r} to "
            "cartopy projection."
        )

greglucas · 2023-07-12T01:55:12Z

lib/cartopy/io/ogc_clients.py

@@ -666,7 +696,17 @@ def __init__(self, service, features, getfeature_extra_kwargs=None):
            raise ImportError(_OWSLIB_REQUIRED)

        if isinstance(service, str):
+            # base url without http:// or https://, and :port


A bit unclear exactly what you're doing here, but can you use urllib's parsing to make this more explicit?
https://docs.python.org/3/library/urllib.parse.html

Thanks. I use urlparse now and added comments.

greglucas · 2023-07-12T02:00:35Z

lib/cartopy/tests/mpl/test_features.py

+@pytest.mark.mpl_image_compare(filename='wfs_france.png')
+def test_wfs_france():
+    ax = plt.axes(projection=ccrs.epsg(2154))
+    url = 'https://agroenvgeo.data.inra.fr:443/geoserver/wfs'


I'm wondering if it would be beneficial for us to start mocking servers rather than relying on external sources? Basically, if it'd be reasonable to add both the XML you download from there and the image to the repository and then make a Mock response from that url. Then we wouldn't need these network marks anymore.

This doesn't have to be undertaken here, it is more me just wondering how reliable these external servers are and if there is a way around hitting them constantly if they don't like that.

We could use vcrpy to record and playback http traffic. That would be a low-cost way to "mock" it without questioning whether there's value in having the mock. It gets messy since the only true measure of "works" is "does it communicate with this resources that's outside our control".

The pytest plugin for VCR looks like an easy solution. However, I leave this to the main developers because it adds new dependencies to cartopy testing.

lib/cartopy/io/ogc_clients.py

greglucas · 2023-07-23T14:01:45Z

lib/cartopy/io/ogc_clients.py

+            with warnings.catch_warnings():
+                warnings.simplefilter("ignore")


What is the warning we are ignoring? We should at least mention why this is here with a comment.

Yes, this is a UserWarning from .to_dict() from proj because the dict does not give all details of each projection. But we only check for the units here.
I added a comment.

greglucas · 2023-07-23T14:17:29Z

lib/cartopy/io/ogc_clients.py

+        # Try to get other geometries from other servers than
+        # http://mapserver.gis.umn.edu/mapserver


Is there a way to add an extra line or two above to make that loop work for your case? i.e. can you add something above to change the tree.findall() call like updating the _MAP_SERVER_NS or just adding an extra block to find all of your geom tags?

This just reads as a lot of copied code, so another option would be to make a function that both loops could use too. Something like this to update your geometry lists that could be re-used in both loops.

`linear_rings_data, linestrings_data, point_data = _get_geometries(node, linear_rings_data, linestrings_data, point_data)

O.K. I was trying to not change existing code. Now it is included in the same loop.

lib/cartopy/io/ogc_clients.py

Added support for general EPSG projections in OGC clients WMSRasterSource, WMTSRasterSource, and WFSGeometrySource. Try to determine geometries from other map servers than the hard-coded mapserver.gis.umn.edu.

greglucas

Looks good, thanks, @mcuntz!

dopplershift added this to the 0.22 milestone Jun 13, 2023

greglucas reviewed Jul 12, 2023

View reviewed changes

greglucas reviewed Jul 23, 2023

View reviewed changes

Enhanced OGC reader

68a6a84

Added support for general EPSG projections in OGC clients WMSRasterSource, WMTSRasterSource, and WFSGeometrySource. Try to determine geometries from other map servers than the hard-coded mapserver.gis.umn.edu.

greglucas approved these changes Jul 24, 2023

View reviewed changes

greglucas merged commit 3af094f into SciTools:main Jul 24, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced OGC reader #2191

Enhanced OGC reader #2191

mcuntz commented Jun 11, 2023

mcuntz commented Jun 11, 2023

mcuntz commented Jun 11, 2023

akrherz commented Jun 11, 2023

greglucas Jul 12, 2023

mcuntz Jul 22, 2023

greglucas Jul 12, 2023

dopplershift Jul 17, 2023

mcuntz Jul 22, 2023

greglucas Jul 23, 2023

mcuntz Jul 23, 2023

greglucas Jul 23, 2023

mcuntz Jul 23, 2023

greglucas left a comment

		with warnings.catch_warnings():
		warnings.simplefilter("ignore")

		# Try to get other geometries from other servers than
		# http://mapserver.gis.umn.edu/mapserver

Enhanced OGC reader #2191

Enhanced OGC reader #2191

Conversation

mcuntz commented Jun 11, 2023

Rationale

Implications

mcuntz commented Jun 11, 2023

mcuntz commented Jun 11, 2023

akrherz commented Jun 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

greglucas left a comment

Choose a reason for hiding this comment